Check default features when skipping resolution of a package

Previously if a package had been activated without the default feature and then
it was attempted to be reactivated with the default feature, resolution wouldn't
continue and actually activate the default feature. This means that the
fingerprint on the other end of compilation would be slightly different because
the set of activated features would be slightly different, causing spurious
recompiles.

Closes #1567

diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
index d3b58c8fb6e70b189f8a8f6a6b29d92d5d7ca499..2f9bd5725fdc2e10aaf35ff01677eafb38e7ae3e 100644 (file)
--- a/src/cargo/core/resolver/mod.rs
+++ b/src/cargo/core/resolver/mod.rs
@@ -112,7 +112,12 @@ impl Resolve {
 
 impl fmt::Debug for Resolve {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        self.graph.fmt(fmt)
+        try!(write!(fmt, "graph: {:?}\n", self.graph));
+        try!(write!(fmt, "\nfeatures: {{\n"));
+        for (pkg, features) in &self.features {
+            try!(write!(fmt, "  {}: {:?}\n", pkg, features));
+        }
+        write!(fmt, "}}")
     }
 }
 
@@ -223,12 +228,17 @@ fn flag_activated(cx: &mut Context,
         return false
     }
     debug!("checking if {} is already activated", summary.package_id());
-    let features = match *method {
-        Method::Required{features, ..} => features,
+    let (features, use_default) = match *method {
+        Method::Required { features, uses_default_features, .. } => {
+            (features, uses_default_features)
+        }
         Method::Everything => return false,
     };
     match cx.resolve.features(id) {
-        Some(prev) => features.iter().all(|f| prev.contains(f)),
+        Some(prev) => {
+            features.iter().all(|f| prev.contains(f)) &&
+                (!use_default || prev.contains("default"))
+        }
         None => features.len() == 0,
     }
 }
@@ -422,7 +432,7 @@ fn resolve_features<'a>(cx: &mut Context, parent: &'a Summary,
                                                (&'a Dependency, Vec<String>)>> {
     let dev_deps = match method {
         Method::Everything => true,
-        Method::Required{dev_deps, ..} => dev_deps,
+        Method::Required { dev_deps, .. } => dev_deps,
     };
 
     // First, filter by dev-dependencies
@@ -519,14 +529,13 @@ fn build_features(s: &Summary, method: Method)
     }
     match method {
         Method::Everything |
-        Method::Required{uses_default_features: true, ..} => {
-            if s.features().get("default").is_some() &&
-               !visited.contains("default") {
+        Method::Required { uses_default_features: true, .. } => {
+            if s.features().get("default").is_some() {
                 try!(add_feature(s, "default", &mut deps, &mut used,
                                  &mut visited));
             }
         }
-        _ => {}
+        Method::Required { uses_default_features: false, .. } => {}
     }
     return Ok((deps, used));
 
@@ -560,9 +569,8 @@ fn build_features(s: &Summary, method: Method)
                 used.insert(feat.to_string());
                 match s.features().get(feat) {
                     Some(recursive) => {
-                        for f in recursive.iter() {
-                            try!(add_feature(s, f, deps, used,
-                                             visited));
+                        for f in recursive {
+                            try!(add_feature(s, f, deps, used, visited));
                         }
                     }
                     None => {

diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs
index 8537974f2a4284a26885448af48e00e3400557b0..707f0ccb9003e732a43beb11de3b39bd52e65758 100644 (file)
--- a/src/cargo/ops/cargo_rustc/fingerprint.rs
+++ b/src/cargo/ops/cargo_rustc/fingerprint.rs
@@ -153,6 +153,7 @@ fn calculate<'a, 'b>(cx: &mut Context<'a, 'b>,
     });
     let extra = util::short_hash(&(cx.config.rustc_version(), target, &features,
                                    profile));
+    debug!("extra {:?} {:?} {:?} = {}", target, profile, features, extra);
 
     // Next, recursively calculate the fingerprint for all of our dependencies.
     //

diff --git a/tests/test_cargo_features.rs b/tests/test_cargo_features.rs
index 10bb898e20cb229321d3a74997d15e2988afa5b6..d46d0d7aec0421621738589f3366e9f6214d4ba7 100644 (file)
--- a/tests/test_cargo_features.rs
+++ b/tests/test_cargo_features.rs
@@ -655,3 +655,43 @@ test!(everything_in_the_lockfile {
     assert!(lockfile.contains(r#"name = "d2""#), "d2 not found\n{}", lockfile);
     assert!(lockfile.contains(r#"name = "d3""#), "d3 not found\n{}", lockfile);
 });
+
+test!(no_rebuild_when_frobbing_default_feature {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            a = { path = "a" }
+            b = { path = "b" }
+        "#)
+        .file("src/lib.rs", "")
+        .file("b/Cargo.toml", r#"
+            [package]
+            name = "b"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            a = { path = "../a", features = ["f1"], default-features = false }
+        "#)
+        .file("b/src/lib.rs", "")
+        .file("a/Cargo.toml", r#"
+            [package]
+            name = "a"
+            version = "0.1.0"
+            authors = []
+
+            [features]
+            default = ["f1"]
+            f1 = []
+        "#)
+        .file("a/src/lib.rs", "");
+
+    assert_that(p.cargo_process("build"), execs().with_status(0));
+    assert_that(p.cargo("build"), execs().with_status(0).with_stdout(""));
+    assert_that(p.cargo("build"), execs().with_status(0).with_stdout(""));
+});